Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle a blank value for the http_proxy host #18073

Merged
merged 1 commit into from
Oct 10, 2018

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Oct 9, 2018

https://bugzilla.redhat.com/show_bug.cgi?id=1637092

We have no way (in the UI) to remove a previously configured http_proxy host
in Settings.yml/advanced settings, so people were forced to save a ''
value. This code was only checking for nil values and should really
check for blank values. It doesn't make sense to reimplement the regex
used by the URI class to parse host values for validity but handling
some obvious blank values make sense here.

https://bugzilla.redhat.com/show_bug.cgi?id=1637092

We have no way (in the UI) to remove a previously configured http_proxy host
in Settings.yml/advanced settings, so people were forced to save a ''
value.  This code was only checking for nil values and should really
check for blank values.  It doesn't make sense to reimplement the regex
used by the URI class to parse host values for validity but handling
some obvious blank values make sense here.
Copy link
Contributor

@yrudman yrudman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 looks good

there would be ability to remove previously configured settings in hammer release ( #17482 and #17563)

@bdunne bdunne merged commit a17d07e into ManageIQ:master Oct 10, 2018
@bdunne bdunne added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 10, 2018
@bdunne bdunne self-assigned this Oct 10, 2018
@jrafanie jrafanie deleted the handle_blank_http_proxy_host branch October 10, 2018 14:35
@jrafanie
Copy link
Member Author

Thanks @yrudman, I updated the bz with links to the bz for the PRs you referenced.

@simaishi
Copy link
Contributor

@jrafanie Can this be hammer/yes and gaprindashvili/yes?

@jrafanie
Copy link
Member Author

Yes, it can.

simaishi pushed a commit that referenced this pull request Oct 15, 2018
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit edcd4ffbb84fc0db15c0ac4f3322da0ba75d0b02
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Wed Oct 10 10:05:50 2018 -0400

    Merge pull request #18073 from jrafanie/handle_blank_http_proxy_host
    
    Handle a blank value for the http_proxy host
    
    (cherry picked from commit a17d07e26daeeb1c06e5dd888ee535ae823cbbe9)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1637092

simaishi pushed a commit that referenced this pull request Nov 5, 2018
@simaishi
Copy link
Contributor

simaishi commented Nov 5, 2018

Hammer backport details:

$ git log -1
commit 44f505db760d4af22f50830863c934e00b78cc18
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Wed Oct 10 10:05:50 2018 -0400

    Merge pull request #18073 from jrafanie/handle_blank_http_proxy_host
    
    Handle a blank value for the http_proxy host
    
    (cherry picked from commit a17d07e26daeeb1c06e5dd888ee535ae823cbbe9)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1639353

nizaminabeel pushed a commit to nizaminabeel/manageiq that referenced this pull request Dec 11, 2018
…xy_host

Handle a blank value for the http_proxy host

(cherry picked from commit a17d07e)

https://bugzilla.redhat.com/show_bug.cgi?id=1639353
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants